Introduce more doc linting #1945
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There are issues in commit 4da38b2: |
|
There are issues in commit e90221e: |
|
There are issues in commit 67a63f7: |
|
There are issues in commit e1db246: |
jnavila
force-pushed
the
doc_linting
branch
8 times, most recently
from
d93f8f0 to
c985eb7
Compare
jnavila
force-pushed
the
doc_linting
branch
4 times, most recently
from
42b2538 to
f2df8e8
Compare
Some readers of man pages have reported that they found malformed linkgit macros in the documentation (absence or bad spelling). Signed-off-by: Jean-Noël Avila <[email protected]>
Having an empty line before each delimited sections is not required by asciidoc, but it is a safety measure that prevents generating malformed asciidoc when generating translated documentation. When a delimited section appears just after a paragraph, the asciidoc processor checks that the length of the delimited section header is different from the length of the paragraph. If it is not, the asciidoc processor will generate a title. In the original English documentation, this is not a problem because the authors always check the output of the asciidoc processor and fix the length of the delimited section header if it turns out to be the same as the paragraph length. However, this is not the case for translations, where the authors have no way to check the length of the delimited section header or the output of the asciidoc processor. This can lead to a section title that is not intended. Indeed, this test also checks that titles are correctly formed, that is, the length of the underline is equal to the length of the title (otherwise it would not be a title but a section header). Finally, this test checks that the delimited section are terminated within the same file. Signed-off-by: Jean-Noël Avila <[email protected]>
|
There are issues in commit 2e88e0f: |
jnavila
force-pushed
the
doc_linting
branch
2 times, most recently
from
56c3c89 to
e03f3f5
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> Reviewing the documentation part of the last patches, it turns out that the
> majority of my comments are related to the latest documentation guidelines
> which are both easy to forget and almost trivial to automatically check.
Thanks. Automation is very much appreciated. These updated
documentation pages will also help as examples when reviewing
others' patches.
Will queue. |
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
![gitgitgadget[bot]](https://avatars.githubusercontent.com/in/12836?s=60&v=4){kind=small}
| $(LINT_DOCS_DELIMITED_SECTIONS): .build/lint-docs/delimited-sections/%.ok: %.adoc | ||
| $(call mkdir_p_parent_template) | ||
| $(QUIET_LINT_DELIMSEC)$(PERL_PATH) lint-delimited-sections.perl $< >$@ | ||
| .PHONY: lint-docs-delimited-sections |
There was a problem hiding this comment.
On the Git mailing list, Ramsay Jones wrote (reply to this):
On 05/08/2025 20:10, Jean-Noël Avila via GitGitGadget wrote:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email protected]>
>
> Due to portability issues, the script generate-configlist.sh was fixed to
> not use carriage returns in the output. However, the result is that it no
> longer correctly handles multiple terms in a single entry of the definition
> list.
>
> We now check that these entries do not exist in the documentation.
>
> Signed-off-by: Jean-Noël Avila <[email protected]>
> ---
> Documentation/Makefile | 10 +++++++++
> Documentation/git-check-attr.adoc | 3 ++-
> Documentation/git-check-ignore.adoc | 9 +++++---
> Documentation/git-http-fetch.adoc | 4 +++-
> Documentation/lint-documentation-style.perl | 24 +++++++++++++++++++++
> Documentation/technical/api-path-walk.adoc | 5 ++++-
> shared.mak | 1 +
> 7 files changed, 50 insertions(+), 6 deletions(-)
> create mode 100755 Documentation/lint-documentation-style.perl
>
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 76a9e1d02b26..ac8a21e3015c 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -508,6 +508,15 @@ $(LINT_DOCS_DELIMITED_SECTIONS): .build/lint-docs/delimited-sections/%.ok: %.ado
> .PHONY: lint-docs-delimited-sections
> lint-docs-delimited-sections: $(LINT_DOCS_DELIMITED_SECTIONS)
>
> +## Lint: Documentation style
> +LINT_DOCS_DOC_STYLE = $(patsubst %.adoc,.build/lint-docs/doc-style/%.ok,$(MAN_TXT))
> +$(LINT_DOCS_DOC_STYLE): lint-documentation-style.perl
> +$(LINT_DOCS_DOC_STYLE): .build/lint-docs/doc-style/%.ok: %.adoc
> + $(call mkdir_p_parent_template)
> + $(QUIET_LINT_DOCSTYLE)$(PERL_PATH) lint-documentation-style.perl $< >$@
> +.PHONY: lint-docs-doc-style
> +lint-docs-doc-style: $(LINT_DOCS_DOC_STYLE)
> +
> lint-docs-manpages:
> $(QUIET_GEN)./lint-manpages.sh
>
> @@ -537,6 +546,7 @@ lint-docs: lint-docs-gitlink
> lint-docs: lint-docs-man-end-blurb
> lint-docs: lint-docs-man-section-order
> lint-docs: lint-docs-delimited-sections
> +lint-docs: lint-docs-doc-style
> lint-docs: lint-docs-manpages
> lint-docs: lint-docs-meson
>
> diff --git a/Documentation/git-check-attr.adoc b/Documentation/git-check-attr.adoc
> index 503b6446574d..15a37a38e3f7 100644
> --- a/Documentation/git-check-attr.adoc
> +++ b/Documentation/git-check-attr.adoc
> @@ -19,7 +19,8 @@ For every pathname, this command will list if each attribute is 'unspecified',
>
> OPTIONS
> -------
> --a, --all::
> +-a::
> +--all::
> List all attributes that are associated with the specified
> paths. If this option is used, then 'unspecified' attributes
> will not be included in the output.
> diff --git a/Documentation/git-check-ignore.adoc b/Documentation/git-check-ignore.adoc
> index 3e3b4e344629..a6c6c1b6e5be 100644
> --- a/Documentation/git-check-ignore.adoc
> +++ b/Documentation/git-check-ignore.adoc
> @@ -25,11 +25,13 @@ subject to exclude rules; but see `--no-index'.
>
> OPTIONS
> -------
> --q, --quiet::
> +-q::
> +--quiet::
> Don't output anything, just set exit status. This is only
> valid with a single pathname.
>
> --v, --verbose::
> +-v::
> +--verbose::
> Instead of printing the paths that are excluded, for each path
> that matches an exclude pattern, print the exclude pattern
> together with the path. (Matching an exclude pattern usually
> @@ -49,7 +51,8 @@ linkgit:gitignore[5].
> below). If `--stdin` is also given, input paths are separated
> with a NUL character instead of a linefeed character.
>
> --n, --non-matching::
> +-n::
> +--non-matching::
> Show given paths which don't match any pattern. This only
> makes sense when `--verbose` is enabled, otherwise it would
> not be possible to distinguish between paths which match a
> diff --git a/Documentation/git-http-fetch.adoc b/Documentation/git-http-fetch.adoc
> index 4ec7c68d3b9e..dcb05890aefd 100644
> --- a/Documentation/git-http-fetch.adoc
> +++ b/Documentation/git-http-fetch.adoc
> @@ -25,8 +25,10 @@ commit-id::
> Either the hash or the filename under [URL]/refs/ to
> pull.
>
> --a, -c, -t::
> +-a::-c::
> +-t::
s/-a::-c::/-a::
-c::/ ?
ATB,
Ramsay Jones
There was a problem hiding this comment.
On the Git mailing list, Jean-Noël AVILA wrote (reply to this):
On Tuesday, 5 August 2025 22:30:23 CEST Ramsay Jones wrote:
> On 05/08/2025 20:10, Jean-Noël Avila via GitGitGadget wrote:
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email protected]>
> >
> > Due to portability issues, the script generate-configlist.sh was fixed to
> > not use carriage returns in the output. However, the result is that it no
> > longer correctly handles multiple terms in a single entry of the
definition
> > list.
> >
> > We now check that these entries do not exist in the documentation.
> >
> > Signed-off-by: Jean-Noël Avila <[email protected]>
> > ---
> >
> > Documentation/Makefile | 10 +++++++++
> > Documentation/git-check-attr.adoc | 3 ++-
> > Documentation/git-check-ignore.adoc | 9 +++++---
> > Documentation/git-http-fetch.adoc | 4 +++-
> > Documentation/lint-documentation-style.perl | 24 +++++++++++++++++++++
> > Documentation/technical/api-path-walk.adoc | 5 ++++-
> > shared.mak | 1 +
> > 7 files changed, 50 insertions(+), 6 deletions(-)
> > create mode 100755 Documentation/lint-documentation-style.perl
> >
> > diff --git a/Documentation/Makefile b/Documentation/Makefile
> > index 76a9e1d02b26..ac8a21e3015c 100644
> > --- a/Documentation/Makefile
> > +++ b/Documentation/Makefile
> > @@ -508,6 +508,15 @@ $(LINT_DOCS_DELIMITED_SECTIONS):
> > .build/lint-docs/delimited-sections/%.ok: %.ado>
> > .PHONY: lint-docs-delimited-sections
> > lint-docs-delimited-sections: $(LINT_DOCS_DELIMITED_SECTIONS)
> >
> > +## Lint: Documentation style
> > +LINT_DOCS_DOC_STYLE = $(patsubst
> > %.adoc,.build/lint-docs/doc-style/%.ok,$(MAN_TXT)) +$
(LINT_DOCS_DOC_STYLE):
> > lint-documentation-style.perl
> > +$(LINT_DOCS_DOC_STYLE): .build/lint-docs/doc-style/%.ok: %.adoc
> > + $(call mkdir_p_parent_template)
> > + $(QUIET_LINT_DOCSTYLE)$(PERL_PATH) lint-documentation-style.perl $<
>$@
> > +.PHONY: lint-docs-doc-style
> > +lint-docs-doc-style: $(LINT_DOCS_DOC_STYLE)
> > +
> >
> > lint-docs-manpages:
> > $(QUIET_GEN)./lint-manpages.sh
> >
> > @@ -537,6 +546,7 @@ lint-docs: lint-docs-gitlink
> >
> > lint-docs: lint-docs-man-end-blurb
> > lint-docs: lint-docs-man-section-order
> > lint-docs: lint-docs-delimited-sections
> >
> > +lint-docs: lint-docs-doc-style
> >
> > lint-docs: lint-docs-manpages
> > lint-docs: lint-docs-meson
> >
> > diff --git a/Documentation/git-check-attr.adoc
> > b/Documentation/git-check-attr.adoc
> > index 503b6446574d..15a37a38e3f7 100644
> > --- a/Documentation/git-check-attr.adoc
> > +++ b/Documentation/git-check-attr.adoc
> > @@ -19,7 +19,8 @@ For every pathname, this command will list if each
attribute is
> > 'unspecified',>
> > OPTIONS
> > -------
> >
> > --a, --all::
> > +-a::
> >
> > +--all::
> > List all attributes that are associated with the specified
> > paths. If this option is used, then 'unspecified' attributes
> > will not be included in the output.
> >
> > diff --git a/Documentation/git-check-ignore.adoc
> > b/Documentation/git-check-ignore.adoc index 3e3b4e344629..a6c6c1b6e5be
100644
> > --- a/Documentation/git-check-ignore.adoc
> > +++ b/Documentation/git-check-ignore.adoc
> > @@ -25,11 +25,13 @@ subject to exclude rules; but see `--no-index'.
> >
> > OPTIONS
> > -------
> >
> > --q, --quiet::
> > +-q::
> >
> > +--quiet::
> > Don't output anything, just set exit status. This is only
> > valid with a single pathname.
> >
> > --v, --verbose::
> > +-v::
> >
> > +--verbose::
> > Instead of printing the paths that are excluded, for each path
> > that matches an exclude pattern, print the exclude pattern
> > together with the path. (Matching an exclude pattern usually
> >
> > @@ -49,7 +51,8 @@ linkgit:gitignore[5].
> >
> > below). If `--stdin` is also given, input paths are separated
> > with a NUL character instead of a linefeed character.
> >
> > --n, --non-matching::
> > +-n::
> >
> > +--non-matching::
> > Show given paths which don't match any pattern. This only
> > makes sense when `--verbose` is enabled, otherwise it would
> > not be possible to distinguish between paths which match a
> >
> > diff --git a/Documentation/git-http-fetch.adoc
> > b/Documentation/git-http-fetch.adoc
> > index 4ec7c68d3b9e..dcb05890aefd 100644
> > --- a/Documentation/git-http-fetch.adoc
> > +++ b/Documentation/git-http-fetch.adoc
> >
> > @@ -25,8 +25,10 @@ commit-id::
> > Either the hash or the filename under [URL]/refs/ to
> > pull.
> >
> > --a, -c, -t::
> > +-a::-c::
>
> > +-t::
> s/-a::-c::/-a::
> -c::/ ?
>
> ATB,
> Ramsay Jones
Yep, the newline is inserted too late.
Thank you for the review. Will reroll in a couple days.
Jean-Noël Avila
|
User |
|
This patch series was integrated into seen via git@8bf3722. |
| $(LINT_DOCS_DELIMITED_SECTIONS): .build/lint-docs/delimited-sections/%.ok: %.adoc | ||
| $(call mkdir_p_parent_template) | ||
| $(QUIET_LINT_DELIMSEC)$(PERL_PATH) lint-delimited-sections.perl $< >$@ | ||
| .PHONY: lint-docs-delimited-sections |
There was a problem hiding this comment.
On the Git mailing list, Collin Funk wrote (reply to this):
Hi,
"Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email protected]>
>
> Due to portability issues, the script generate-configlist.sh was fixed to
> not use carriage returns in the output. However, the result is that it no
> longer correctly handles multiple terms in a single entry of the definition
> list.
>
> We now check that these entries do not exist in the documentation.
>
> Signed-off-by: Jean-Noël Avila <[email protected]>
> ---
> Documentation/Makefile | 10 +++++++++
> Documentation/git-check-attr.adoc | 3 ++-
> Documentation/git-check-ignore.adoc | 9 +++++---
> Documentation/git-http-fetch.adoc | 4 +++-
> Documentation/lint-documentation-style.perl | 24 +++++++++++++++++++++
> Documentation/technical/api-path-walk.adoc | 5 ++++-
> shared.mak | 1 +
> 7 files changed, 50 insertions(+), 6 deletions(-)
> create mode 100755 Documentation/lint-documentation-style.perl
I documented that this was the correct way to format them in
CodingGuidelines. At the time I commented that there were some places
that didn't follow this rule. Junio replied [1]:
> We are updating them gradually while avoiding collisions with
> patches that do other "real" work; see many recent patches to
> Documentation/config/ area by Jean-Noël Avila for more, e.g.
> d30c5cc4 (doc: convert git-mergetool options to new synopsis style,
> 2025-05-25).
As long as he is okay with the change, this looks good to me. It isn't
that many changes, so hopefully it is. :)
Small nit, but the issue was '\n' not being interpreted as a newline in
sed's s command. Mentioning carriage return makes me think of '\r'.
Reviewed-by: Collin Funk <[email protected]>
CollinThere was a problem hiding this comment.
On the Git mailing list, Jean-Noël AVILA wrote (reply to this):
On Wednesday, 6 August 2025 03:02:04 CEST Collin Funk wrote:
> Hi,
>
> "Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> > From: =?UTF-8?q?Jean-No=C3=ABl=20Avila?= <[email protected]>
> >
> > Due to portability issues, the script generate-configlist.sh was fixed to
> > not use carriage returns in the output. However, the result is that it no
> > longer correctly handles multiple terms in a single entry of the
definition
> > list.
> >
> > We now check that these entries do not exist in the documentation.
> >
> > Signed-off-by: Jean-Noël Avila <[email protected]>
> > ---
> >
> > Documentation/Makefile | 10 +++++++++
> > Documentation/git-check-attr.adoc | 3 ++-
> > Documentation/git-check-ignore.adoc | 9 +++++---
> > Documentation/git-http-fetch.adoc | 4 +++-
> > Documentation/lint-documentation-style.perl | 24 +++++++++++++++++++++
> > Documentation/technical/api-path-walk.adoc | 5 ++++-
> > shared.mak | 1 +
> > 7 files changed, 50 insertions(+), 6 deletions(-)
> > create mode 100755 Documentation/lint-documentation-style.perl
>
> I documented that this was the correct way to format them in
> CodingGuidelines. At the time I commented that there were some places
>
> that didn't follow this rule. Junio replied [1]:
> > We are updating them gradually while avoiding collisions with
> > patches that do other "real" work; see many recent patches to
> > Documentation/config/ area by Jean-Noël Avila for more, e.g.
> > d30c5cc4 (doc: convert git-mergetool options to new synopsis style,
> > 2025-05-25).
>
> As long as he is okay with the change, this looks good to me. It isn't
> that many changes, so hopefully it is. :)
>
> Small nit, but the issue was '\n' not being interpreted as a newline in
> sed's s command. Mentioning carriage return makes me think of '\r'.
>
> Reviewed-by: Collin Funk <[email protected]>
>
> Collin
As a matter of fact, the script did not check config description files, but
only root man pages. Will also push this change in the next iteration.
Thanks
Jean-Noël
|
User |
|
This patch series was integrated into seen via git@df50034. |
|
This patch series was integrated into seen via git@990db2f. |
|
This patch series was integrated into seen via git@0f181a9. |
For simplifying automated translation of the documentation, it is better to only present one term in each entry of a description list of options. This is because most of these terms can automatically be marked as notranslatable. Also, due to portability issues, the script generate-configlist.sh can no longer insert newlines in the output. However, the result is that it no longer correctly handles multiple terms in a single entry of definition lists. As a result, we now check that these entries do not exist in the documentation. Reviewed-by: Collin Funk <[email protected]> Signed-off-by: Jean-Noël Avila <[email protected]>
For better searchability, this commit adds a check to ensure that parameters expressed in the form of `--[no-]parameter` are not used in the documentation. In the place of such parameters, the documentation should list two separate parameters: `--parameter` and `--no-parameter`. Signed-off-by: Jean-Noël Avila <[email protected]>
This commit fixes the synopsis syntax and changes the wording of a few descriptions to be more consistent with the rest of the documentation. It is a prepartion for the next commit that checks that synopsis style is applied consistently across a manual page. Signed-off-by: Jean-Noël Avila <[email protected]>
When switching manpages to the synopsis style, the description lists of options need to be switched to inline synopsis for proper formatting. This is done by enclosing the option name in double backticks, e.g. `--option`. Signed-off-by: Jean-Noël Avila <[email protected]>
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
This patch series was integrated into seen via git@d81a1a9. |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> Reviewing the documentation part of the last patches, it turns out that the
> majority of my comments are related to the latest documentation guidelines
> which are both easy to forget and almost trivial to automatically check.
>
> This series implements the automatic tests for basic doc rules. At the
> moment it conflicts with "[GSoC][PATCH v6 0/6] Add refs list subcommand" and
> possibly with "[PATCH v4 0/9] refs: fix migration of reflog entries"
>
> Changes since v1:
>
> * fix a small typo
>
> Changes since v2:
>
> * extend range of check files for multiple entries in definition list
> entries
> * extend checks for new synopsis styles
This round has been in 'seen', hasn't triggered any false positives.
Let's mark the topic for 'next'? |
|
On the Git mailing list, Jean-Noël AVILA wrote (reply to this): On Thursday, 14 August 2025 18:34:47 CEST Junio C Hamano wrote:
> "Jean-Noël Avila via GitGitGadget" <[email protected]> writes:
> > Reviewing the documentation part of the last patches, it turns out that
the
> > majority of my comments are related to the latest documentation guidelines
> > which are both easy to forget and almost trivial to automatically check.
> >
> > This series implements the automatic tests for basic doc rules. At the
> > moment it conflicts with "[GSoC][PATCH v6 0/6] Add refs list subcommand"
and
> > possibly with "[PATCH v4 0/9] refs: fix migration of reflog entries"
> >
> > Changes since v1:
> > * fix a small typo
> >
> > Changes since v2:
> > * extend range of check files for multiple entries in definition list
> >
> > entries
> >
> > * extend checks for new synopsis styles
>
> This round has been in 'seen', hasn't triggered any false positives.
> Let's mark the topic for 'next'?
I'm OK with this.
|
|
This branch is now known as |
|
This patch series was integrated into seen via git@c94303b. |
|
This patch series was integrated into seen via git@9cd6ead. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reviewing the documentation part of the last patches, it turns out that the majority of my comments are related to the latest documentation guidelines which are both easy to forget and almost trivial to automatically check.
This series implements the automatic tests for basic doc rules. At the moment it conflicts with "[GSoC][PATCH v6 0/6] Add refs list subcommand" and possibly with "[PATCH v4 0/9] refs: fix migration of reflog entries"
Changes since v1:
Changes since v2:
cc: Ramsay Jones [email protected]
cc: Collin Funk [email protected]